Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hotfix to hide the external link badge from cards #1231

Merged
merged 3 commits into from
Nov 5, 2024

Conversation

dzole0311
Copy link
Collaborator

Related Ticket: {link related ticket here}

Description of Changes

This update addresses a mistake I made from the initial fix involving the new isExternalLink prop, which unintentionally altered the behavior of external links. Basically, this prop not only hid the "External Link" badge but also incorrectly routed external links through React's internal routing, resulting in URLs like earth.gov/visit/https://earth.gov/mobile-climate-mapper instead of correctly navigating to earth.gov/mobile-climate-mapper

What's fixed here:

  1. Removed the isExternalLink prop to revert unintended internal link behavior
  2. Introduced a new prop, hideExternalLinkBadge that more clearly describes it's role and is used specifically for visually hiding the "External Link" badge without altering the link's external behavior

Notes & Questions About Changes

To minimize team effort for a hotfix release and since this fix is specific to the EIC instance only, I did the following:

  1. Created a branch from the latest tag specifically for this hotfix
  2. Bumped the EIC instance to the latest UI version using this hotfix branch
  3. Protected the hotfix branch, which will remain in place until the next planned release (in two weeks)

Validation / Testing

  1. Use the hideExternalLinkBadge prop to the External Link Test story
  2. Check that the "External Link" badge is hidden from the story card
  3. Check that the link is opened correctly once the card is clicked

Or check it out in practice on the EIC instance here, where the Mobile Climate Mapper card has no "External Link" badge anymore:

Copy link

netlify bot commented Oct 31, 2024

Deploy Preview for veda-ui ready!

Name Link
🔨 Latest commit e9b231b
🔍 Latest deploy log https://app.netlify.com/sites/veda-ui/deploys/6723ab689c59d80008323c4a
😎 Deploy Preview https://deploy-preview-1231--veda-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

};
}

const isExternalLink = linkProperties.isLinkExternal ?? /^https?:\/\//.test(linkProperties.linkTo);
const isExternalLink = /^https?:\/\//.test(linkProperties.linkTo);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes! this is what we all needed. Thanks for thinking through this again 🙇

@sandrahoang686
Copy link
Collaborator

@dzole0311 if you created the 5.9.1 patch from here, could you update the package.json too to when it merges, it has the udpate version reflected?

@dzole0311 dzole0311 merged commit 4d4b9b4 into main Nov 5, 2024
9 checks passed
@AliceR AliceR mentioned this pull request Nov 11, 2024
AliceR added a commit that referenced this pull request Nov 12, 2024
## 🎉 Features
* Card image/description independent of hero image/description by
@dzole0311 in #1244

## 🚀 Improvements
* Cookie consent code cleanup by @snmln in
#1199 , and @hanbyul-here in
#1240 , and @hanbyul-here in
#1241
* Add ADR about design system change by @j08lue in
#890
* Update condition to run playwright tests on release branches by
@dzole0311 in #1228
* Update STYLE_GUIDE.md by @AliceR in
#1227
* Fix lint configuration by @AliceR in
#1219
* Add tests for the AOI feature specification by @AliceR in
#1216
* Set data catalog filters to be closed by default by @vgeorge in
#1243
* Update tsconfig and make nav interfaces exposable for consumption by
@sandrahoang686 in #1223

## 🐛 Fixes
* Hotfix to hide the external link badge from cards by @dzole0311 in
#1231

## New Contributors
* @vgeorge made their first contribution in
#1243

**Full Changelog**:
v5.9.0...v5.10.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants